Skip to content

POSTabEligibilityCheckerI2: i2 logic #15865

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 8 commits into from
Jul 4, 2025
Merged

POSTabEligibilityCheckerI2: i2 logic #15865

merged 8 commits into from
Jul 4, 2025

Conversation

@jaclync
Copy link
Contributor

@jaclync jaclync commented Jul 3, 2025 •
edited
Loading

Part 2 of WOOMOB-756

Just one review is required.

⚠️ Please only review this PR if the base PR #15864 is approved without a need for significant changes.

Apology for the large diffs, the test file is ~670 diffs with some duplication of the test helpers. When pointOfSaleAsATabi2 feature flag is removed, POSTabEligibilityCheckerI2 and POSTabEligibilityCheckerI2Tests will replace POSTabEligibilityChecker and POSTabEligibilityCheckerTests.

Description

A recap of the differences between i1 and i2:

milestone POS tab visibility check POS eligibility check
i1 all conditions: tablet, iOS 17+, store country, store currency, remote feature flag, WC plugin version, POS feature switch for WC version 10.0+ none
i2 tablet, store country, remote feature flag iOS 17+, store currency, WC plugin version, POS feature switch for WC version 10.0+

As the base PR #15864 updated POSTabEligibilityChecker and POSTabEligibilityCheckerTests to be i1 only, this PR adds POSTabEligibilityCheckerI2 and POSTabEligibilityCheckerI2Tests for the i2 implementation.

  • POSTabEligibilityCheckerI2: the visibility & eligibility checks should only check for the conditions in the table above. Previously, the eligibility check still included some conditions in the visibility check.
  • POSTabEligibilityCheckerI2Tests: similar to POSTabEligibilityCheckerTests, but all test cases are now for i2.

Steps to reproduce

i2 is back now, and the POS tab is now shown when: on a tablet, store country is eligible, remote feature flag is enabled

Store eligible for POS

  • Log in to a store eligible for POS --> the POS tab should be shown after a bit
  • Tap on the POS tab --> POS should be launched to the products loaded state

Store in an eligible country but not eligible for POS

  • Log in to a store in an eligible country but not eligible for POS --> the POS tab should be shown after a bit
  • Tap on the POS tab --> an ineligible UI should be shown

Store not in an eligible country

  • Log in to a store not in an eligible country --> the POS tab should not be shown even after a while

Testing information

  • @jaclync smoke tests when the i2 feature flag is disabled
  • @jaclync makes sure the POS tab does not show on iPhone

Screenshots

Ineligible UI is back for a store in an eligible country but not eligible for POS:

  • I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

@jaclync jaclync changed the base branch from trunk to feat/WOOMOB-756-separate-ineligible-enum-to-visibility-and-eligibility July 3, 2025 20:44
@jaclync jaclync changed the title Clean up POSTabEligibilityCheckerI2Tests: remove i1 logic and test arguments [POS as a tab i2] Add POSTabEligibilityCheckerI2 and POSTabEligibilityCheckerI2Tests for i2 implementation in a separate class Jul 3, 2025
@wpmobilebot
Copy link
Collaborator

wpmobilebot commented Jul 3, 2025 •
edited
Loading

App Icon📲 You can test the changes from this Pull Request in WooCommerce iOS Prototype by scanning the QR code below to install the corresponding build.

App NameWooCommerce iOS Prototype
Build Numberpr15865-b6854d4
Version22.7
Bundle IDcom.automattic.alpha.woocommerce
Commitb6854d4
Installation URL0gfitedngj0h0
 Automatticians: You can use our internal self-serve MC tool to give yourself access to those builds if needed.

@jaclync jaclync added type: task An internally driven task. feature: POS labels Jul 3, 2025
@jaclync jaclync added this to the 22.8 milestone Jul 3, 2025
@jaclync jaclync changed the title [POS as a tab i2] Add POSTabEligibilityCheckerI2 and POSTabEligibilityCheckerI2Tests for i2 implementation in a separate class Clean up POSTabEligibilityCheckerI2Tests: remove i1 logic and test arguments Jul 4, 2025
@joshheald joshheald self-assigned this Jul 4, 2025
joshheald
joshheald approved these changes Jul 4, 2025
View reviewed changes
Copy link
Contributor

@joshheald joshheald left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works as expected.

I mentioned renaming a duplicate, in my last PR, but now that you've done it the other way around I don't think it's worth undoing and redoing for a small benefit in source control history.

Thanks for the change.

WooCommerce/Classes/ViewRelated/Dashboard/Settings/POS/POSTabEligibilityCheckerI2.swift Outdated
@@ -0,0 +1,260 @@
import Combine
Copy link
Contributor

@joshheald joshheald Jul 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we're using Combine here

Copy link
Contributor Author

@jaclync jaclync Jul 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch, removed in b6854d4.

@jaclync jaclync changed the title Clean up POSTabEligibilityCheckerI2Tests: remove i1 logic and test arguments Clean up POSTabEligibilityCheckerI: remove i1 logic Jul 4, 2025
@jaclync jaclync changed the title Clean up POSTabEligibilityCheckerI: remove i1 logic POSTabEligibilityCheckerI2: i2 logic Jul 4, 2025
@jaclync jaclync mentioned this pull request Jul 4, 2025
Merged
4 tasks
jaclync added 4 commits July 4, 2025 15:19
@jaclync
Merge branch 'feat/WOOMOB-756-separate-ineligible-enum-to-visibility-…
2e21fc4 
…and-eligibility' into feat/WOOMOB-756-eligibility-checker-i2

# Conflicts:
#	WooCommerce/Classes/ViewRelated/MainTabBarController.swift
#	WooCommerce/WooCommerce.xcodeproj/project.pbxproj
@jaclync
Move i2 content (checker and tests) to POSTabEligibilityChecker and…
b06a0f5 
… `POSTabEligibilityCheckerTests`.
@jaclync
Remove POSIneligibleReason cases that aren't in i2.
297742d
@jaclync
POSTabEligibilityChecker: remove unused Combine import.
b6854d4
Base automatically changed from feat/WOOMOB-756-separate-ineligible-enum-to-visibility-and-eligibility to trunk July 4, 2025 19:37
@jaclync
Copy link
Contributor Author

jaclync commented Jul 4, 2025

Thanks for reviewing both PRs Josh, I've copied the i2 content to the main checker & tests files, with i2 files deleted now. Merging this to unblock future PRs.

@jaclync jaclync enabled auto-merge July 4, 2025 19:44
@jaclync jaclync merged commit ca67d0f into trunk Jul 4, 2025
13 of 14 checks passed
@jaclync jaclync deleted the feat/WOOMOB-756-eligibility-checker-i2 branch July 4, 2025 19:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@joshheald joshheald joshheald approved these changes

@iamgabrielma iamgabrielma Awaiting requested review from iamgabrielma

@staskus staskus Awaiting requested review from staskus

Assignees

@joshheald joshheald

Labels

feature: POS type: task An internally driven task.

Projects

None yet

Milestone

22.8

Development

Successfully merging this pull request may close these issues.

4 participants

@jaclync @wpmobilebot @joshheald